Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

xip_ram_perms/CMakeLists.txt: unset environment variables CFLAGS, CXX… #140

Merged
merged 1 commit into from
Sep 18, 2024
Merged

xip_ram_perms/CMakeLists.txt: unset environment variables CFLAGS, CXX… #140

merged 1 commit into from
Sep 18, 2024

Conversation

josch
Copy link

@josch josch commented Sep 9, 2024

…FLAGS and LDFLAGS

If the user set these environment variables to influence the picotool build, unset them here so that they do not influence the pico-sdk build. This is especially required for flags that are not supported by arm-none-eabi compilers.

As suggested by @will-v-pi in #139 (comment)

@will-v-pi
Copy link

Could you target this at the develop branch rather than master?

Also, I think the same changes should probably be added the the picoboot_flash_id/CMakeLists.txt file as well, as that also uses a pico-sdk build

@josch
{xip_ram_perms,picoboot_flash_id}/CMakeLists.txt: unset environment v…
4d54505 
…ariables CFLAGS, CXXFLAGS and LDFLAGS

If the user set these environment variables to influence the picotool
build, unset them here so that they do not influence the pico-sdk
build. This is especially required for flags that are not supported
by arm-none-eabi compilers.
@josch josch changed the base branch from master to develop September 9, 2024 14:03
@lurch
Copy link
Contributor

lurch commented Sep 9, 2024

What if the user wants to set different CFLAGS for the picotool build and the xip_ram_perms build? Would it be worth supporting e.g. a PICO_SDK_CFLAGS setting, which would be used as CFLAGS when building xip_ram_perms and picoboot_flash_id ? 🤷 (Apologies in advance if this is a stupid question!)

@josch josch mentioned this pull request Sep 9, 2024
Closed
@will-v-pi
Copy link

What if the user wants to set different CFLAGS for the picotool build and the xip_ram_perms build? Would it be worth supporting e.g. a PICO_SDK_CFLAGS setting, which would be used as CFLAGS when building xip_ram_perms and picoboot_flash_id ? 🤷 (Apologies in advance if this is a stupid question!)

I can’t really think of a use case for that, as the SDK sets all the relevant CFLAGS required to build correctly? Especially for a minimal binary like this, I’m not sure what extra CFLAGS might be required. So I think we can merge this as is, and revisit if anyone comes up with a need for a PICO_SDK_CFLAGS setting

@lurch
Copy link
Contributor

lurch commented Sep 9, 2024

I can’t really think of a use case for that, as the SDK sets all the relevant CFLAGS required to build correctly?

Fair enough, I guess I was just playing devil's advocate 😆

@will-v-pi will-v-pi modified the milestones: 2.1.0, 2.0.1 Sep 11, 2024
@will-v-pi will-v-pi merged commit 7e2f756 into raspberrypi:develop Sep 18, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@josch @will-v-pi @lurch